Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

out_loki: fix removing keys defined by labels #9766

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajantti
Copy link

@ajantti ajantti commented Dec 25, 2024

When key is defined as a Loki label with labels, label_keys or label_map_path, the code is trying to remove them. However, if remove_keys is not defined, it never gets to the part that actually removes them. This looks like an obvious bug to me.

I moved the debug message to inside the if (size > 0) statement, because earlier it would only print it if it was actually trying to remove something. So without moving it it would spam it every time. And absence of the message would then indicate size == 0. Not sure if this is the best, though.

I moved the code that removes the keys to outside the if (ctx->remove_keys) statement.


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Example configuration file for the change
  • Debug log output from testing the change
  • Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • Run local packaging test showing all targets (including any new ones) build.
  • Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

fluent/fluent-bit-docs#1536

Backporting

  • [N/A] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

When key is defined as a Loki label with labels, label_keys or
label_map_path, the code is trying to remove them. However, if
remove_keys is not defined, it never gets to the part that actually
removes them. This looks like an obvious bug to me.

I moved the debug message to inside the if (size > 0) statement, because
earlier it would only print it if it was actually trying to remove
something. So without moving it it would spam it every time. And absence
of the message would then indicate size == 0. Not sure if this is the
best, though.
@ajantti
Copy link
Author

ajantti commented Dec 25, 2024

Example Configuration

service:
  flush: 1
  log_level: debug
  http_server: true
  http_listen: 0.0.0.0
  http_port: 2020
pipeline:
  inputs:
    - name: systemd
      tag: host.*
      read_from_tail: true
  outputs:
    - name: loki
      host: 127.0.0.1
      port: 3100
      match: '*'
      labels: $MESSAGE

@ajantti
Copy link
Author

ajantti commented Dec 25, 2024

Valgrind/Debug output

==928693== Memcheck, a memory error detector
==928693== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==928693== Using Valgrind-3.23.0 and LibVEX; rerun with -h for copyright info
==928693== Command: /usr/local/bin/fluent-bit -c fluent-bit-pull-request.yaml
==928693==
Fluent Bit v3.2.4
* Copyright (C) 2015-2024 The Fluent Bit Authors
* Fluent Bit is a CNCF sub-project under the umbrella of Fluentd
* https://fluentbit.io

______ _                  _    ______ _ _           _____  _____
|  ___| |                | |   | ___ (_) |         |____ |/ __  \
| |_  | |_   _  ___ _ __ | |_  | |_/ /_| |_  __   __   / /`' / /'
|  _| | | | | |/ _ \ '_ \| __| | ___ \ | __| \ \ / /   \ \  / /
| |   | | |_| |  __/ | | | |_  | |_/ / | |_   \ V /.___/ /./ /___
\_|   |_|\__,_|\___|_| |_|\__| \____/|_|\__|   \_/ \____(_)_____/


[2024/12/25 15:40:57] [ info] Configuration:
[2024/12/25 15:40:57] [ info]  flush time     | 1.000000 seconds
[2024/12/25 15:40:57] [ info]  grace          | 5 seconds
[2024/12/25 15:40:57] [ info]  daemon         | 0
[2024/12/25 15:40:57] [ info] ___________
[2024/12/25 15:40:57] [ info]  inputs:
[2024/12/25 15:40:57] [ info]      systemd
[2024/12/25 15:40:57] [ info] ___________
[2024/12/25 15:40:57] [ info]  filters:
[2024/12/25 15:40:57] [ info] ___________
[2024/12/25 15:40:57] [ info]  outputs:
[2024/12/25 15:40:57] [ info]      loki.0
[2024/12/25 15:40:57] [ info] ___________
[2024/12/25 15:40:57] [ info]  collectors:
[2024/12/25 15:40:57] [ info] [fluent bit] version=3.2.4, commit=c53b61c0d6, pid=928693
[2024/12/25 15:40:57] [debug] [engine] coroutine stack size: 24576 bytes (24.0K)
[2024/12/25 15:40:57] [ info] [storage] ver=1.5.2, type=memory, sync=normal, checksum=off, max_chunks_up=128
[2024/12/25 15:40:57] [ info] [simd    ] disabled
[2024/12/25 15:40:57] [ info] [cmetrics] version=0.9.9
[2024/12/25 15:40:57] [ info] [ctraces ] version=0.5.7
[2024/12/25 15:40:57] [ info] [input:systemd:systemd.0] initializing
[2024/12/25 15:40:57] [ info] [input:systemd:systemd.0] storage_strategy='memory' (memory only)
[2024/12/25 15:40:57] [debug] [systemd:systemd.0] created event channels: read=25 write=26
[2024/12/25 15:40:57] [debug] [input:systemd:systemd.0] jump to the end of journal and skip 0 last entries
[2024/12/25 15:40:57] [debug] [input:systemd:systemd.0] sd_journal library may truncate values to sd_journal_get_data_threshold() bytes: 65536
[2024/12/25 15:40:57] [debug] [loki:loki.0] created event channels: read=30 write=34
[2024/12/25 15:40:57] [debug] [output:loki:loki.0] remove_mpa size: 1
[2024/12/25 15:40:57] [ info] [output:loki:loki.0] configured, hostname=127.0.0.1:3100
[2024/12/25 15:40:57] [ info] [http_server] listen iface=0.0.0.0 tcp_port=2020
[2024/12/25 15:40:57] [ info] [sp] stream processor started
[2024/12/25 15:41:09] [debug] [task] created task=0x5881a30 id=0 OK
[2024/12/25 15:41:09] [debug] [task] created task=0x5881c50 id=1 OK
[2024/12/25 15:41:09] [debug] [upstream] KA connection #53 to 127.0.0.1:3100 is connected
[2024/12/25 15:41:09] [debug] [http_client] not using http_proxy for header
[2024/12/25 15:41:09] [debug] [output:loki:loki.0] 127.0.0.1:3100, HTTP status=204
[2024/12/25 15:41:09] [debug] [upstream] KA connection #53 to 127.0.0.1:3100 is now available
[2024/12/25 15:41:09] [debug] [upstream] KA connection #53 to 127.0.0.1:3100 has been assigned (recycled)
[2024/12/25 15:41:09] [debug] [http_client] not using http_proxy for header
[2024/12/25 15:41:09] [debug] [output:loki:loki.0] 127.0.0.1:3100, HTTP status=204
[2024/12/25 15:41:09] [debug] [upstream] KA connection #53 to 127.0.0.1:3100 is now available
[2024/12/25 15:41:09] [debug] [out flush] cb_destroy coro_id=0
[2024/12/25 15:41:09] [debug] [out flush] cb_destroy coro_id=1
[2024/12/25 15:41:09] [debug] [task] destroy task=0x5881a30 (task_id=0)
[2024/12/25 15:41:09] [debug] [task] destroy task=0x5881c50 (task_id=1)
^C[2024/12/25 15:41:20] [engine] caught signal (SIGINT)
[2024/12/25 15:41:20] [ warn] [engine] service will shutdown in max 5 seconds
[2024/12/25 15:41:20] [ info] [input] pausing systemd.0
[2024/12/25 15:41:21] [ info] [engine] service has stopped (0 pending tasks)
[2024/12/25 15:41:21] [ info] [input] pausing systemd.0
==928693==
==928693== HEAP SUMMARY:
==928693==     in use at exit: 0 bytes in 0 blocks
==928693==   total heap usage: 33,028 allocs, 33,028 frees, 3,881,675 bytes allocated
==928693==
==928693== All heap blocks were freed -- no leaks are possible
==928693==
==928693== For lists of detected and suppressed errors, rerun with: -s
==928693== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

ajantti added a commit to ajantti/fluent-bit-docs that referenced this pull request Dec 25, 2024
Related to the pull request fluent/fluent-bit#9766
The documentation doesn't make it clear, but the code seems to be
trying to work this way.

Also minor typo fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant